Fix for TPC edge clusters in CTF decoding#14161
Conversation
|
REQUEST FOR PRODUCTION RELEASES: This will add The following labels are available |
| } | ||
| const auto cluster = decompressTrackStore(cmprClusters, clusterOffset, slice, row, pad, time, args...); | ||
| auto cluster = decompressTrackStore(cmprClusters, clusterOffset, slice, row, pad, time, args...); | ||
| if ((cluster.getFlags() & ClusterNative::flagEdge) && param.rec.tpc.clustersShiftTimebins != 0.f) { |
There was a problem hiding this comment.
I assume this is wrong, instead of param.rec.tpc.clustersShiftTimebins you'd need to use param.rec.tpc.clustersEdgeFixDistance.
But then, why do you need this change in the TPCClusterDecompressionCore.inc at all, if you fix the clusters later in GPUTPCDecompressionKernels and TPCClusterDecompressor? I'd actually remove this part here, I believe it is not needed, and it adds an additional branch on the GPU if clustersEdgeFixDistance is not set.
There was a problem hiding this comment.
I thought this decompressTrack( is called for attached clusters, in https://github.com/AliceO2Group/AliceO2/blob/dev/GPU/GPUTracking/DataCompression/GPUTPCDecompressionKernels.cxx#L36, while the previous step1unattached is called for unattached clusters in GPU/GPUTracking/Global/GPUChainTrackingCompression.cxx
Please confirm that this is not used.
There was a problem hiding this comment.
ok, this is a bit more convoluted than I thought and when checking again at first I thought you were right and took me one hour to understand...
So, the decompressTrack( is of course used as you say, but the clusters are copied and touched again in the step for unattached clusters here
Note that the loop in these places runs from 0 to nClusters, not only over the unattached ones, so these 2 loops apply the corrections for all clusters.
The
TPCClusterDecompressor is for the tpcUseOldCPUDecoding, while the GPUTPCDecompressionKernels is for Gabriele`s new GPU-capable decoding (which runs on both CPU and GPU).So fixing these 2 places is enough as I wrote initially.
In your case, you are fixing the edge pad during the attached decoding, and fixing it afterwards again.
Also, your approach is bogus, since decompressTrackStore is not guaranteed to return a reference (it does in some of the overloads). I.e., you might be fixing a temporary object. For your approach, you'd need to fix the edge flag before line 140 here
That said, I am not sure if Gabriele or we ever tested the clustersShiftTimebins parameter for the new decoding. Perhaps we should double-check before we rely on it, and try with tpcUseOldCPUDecoding=1 on CPU and with tpcUseOldCPUDecoding=0 on CPU and GPU.
|
Error while checking build/O2/fullCI_slc9 for 2fad56c at 2025-04-10 06:38: Full log here. |
|
@davidrohr ok, removed (1) Both timeshift and edge bit fix work:
|
|
great, thx for doing these checks! |
|
Error while checking build/O2/fullCI_slc9 for 29796fd at 2025-04-10 16:24: Full log here. |
3 participants
Edge clusters before and after the fix with
GPU_rec_tpc.clustersEdgeFixDistance=0.5Default
GPU_rec_tpc.clustersEdgeFixDistanceis 0 (no fix).